ext/intl: various optimization#22069
Conversation
|
If you have further optimisations plan you can put it there too. |
LamentXU123
changed the title
array_init_size instead of array_init when the size is known
LamentXU123
force-pushed
the
opt-intl
branch
3 times, most recently
from
0018bf2 to
da1313e
Compare
|
Now this should be fine to review |
| INTL_CHECK_STATUS(icuerror, "Cannot fetch locales list"); | ||
|
|
||
| count = uenum_count(icuenum, &icuerror); | ||
| if (U_FAILURE(icuerror)) { |
There was a problem hiding this comment.
are you mixing bug fixes with optim ?
There was a problem hiding this comment.
also I doubt a bit more this specific optimisation. i.e. how icuenum->count callback costly is.
| uenum_close(en); | ||
| INTL_CHECK_STATUS(status, "Failed to count registered transliterators"); | ||
| } | ||
| uenum_reset(en, &status); |
There was a problem hiding this comment.
it might be more harmful here than in the other case, you re paying for something that is not necessary, i.e. the enum is already at offset 0.
| } | ||
| uenum_reset( icuenum, &icuerror ); | ||
| INTL_CHECK_STATUS(icuerror, "Cannot iterate locales list"); | ||
| if (U_FAILURE(icuerror)) { |
There was a problem hiding this comment.
please take out those bug fixes and make another PR out of it. clearly a mem leak fix.
|
Turns out the pattern of calculate array length in bulk and use array_init_size doesn't have obvious performance enhance (tested) and it also is a footgun we want to prevent, so I am dropping those. |
2 participants
errorsarray with the correct size infill_errors()#21560 and ext/openssl: various arrays optimisations. #18377In transliterator/transliterator_methods.cpp: counted the ICU enumeration up front withuenum_count(), reset it, and switchedtransliterator_list_ids()toarray_init_size(). So we now use ICU APIs to get total length and use array_init_size instead of array_init directly here.In resourcebundle/resourcebundle_class.cpp: ditto. counted available locales before iteration and initialized the return array with the exact capacity.